Repository navigation
Conversation
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
14df6bd to
0cd6d72
Compare
|
@vogella Since you asked me to create this PR, is there anything else we should do? |
I review next weeek, ping me if I forget. Check, if possible the build error in betweeen and try to fix. |
Looking at https://github.com/eclipse-platform/eclipse.platform/runs/111035168999, it's 503 errors from GitHub (unless something else happened in Jenkins which I don't have the permission to see). I'll do another force push to trigger a rebuild. |
39a283e to
529c68f
Compare
|
@vogella Here's your reminder ping (even though it's just the middle of the week). |
Thanks |
|
Sorry for the review delay. I tried it on Windows 11: a custom PowerShell path with arguments is saved and used for new local terminals, and Restore Defaults brings back A few points:
|
Done (this change also applies to Linux).
Do you think having two sets of preference keys for doing the same thing would be better? Alternatively, I could rename the constants and keep the actual preference names (possibly with a comment)?
Yes, this is the same on Linux. If that should be changed, would another PR be better (since that is kinda new functionality)? If not, do you suggest just checking the
I updated the Javadoc and kept the structure the same. The indentation of that code is still different due to me removing the condition and I had to rename some variables to avoid duplicates. If wanted, I could also keep it in a block ( |
5117195 to
91f5032
Compare
|
(force pushing again because the build failed due to a timeout) |
cd2a06c to
7a686bc
Compare
This PR allows configuring the default shell on Windows and not just on Linux/macOS.
I mainly just removed some
!Platform.OS_WIN32.equals(Platform.getOS())checks where this is relevant (increateContents(Composite), I extracted that part to a method but I can change that back to make it easier to review).Note: For updating the
Leave the shell command empty to fallback to the SHELL environment variable or if not set, to /bin/sh.message for Windows, I duplicated the following code from theorg.eclipse.terminal.connector.localmodule. Alternatively, I could createpublicmethods in thex-internalpackage.Note: When I tried to use it with PowerShell, the color mapping wasn't ideal with some of the text being invisible due to having the same color as the background (PowerShell seems to use the "Bright White" and "Bright Yellow" colors). I don't know whether this is something that can/should be changed. With the "Eclipse Light" color preset, PowerShell was more readable IMO.
Also, I didn't do an exhaustive test of all related code on Windows but it seems like the default configuration, changing the shell/arguments and restoring the defaults works.
I don't know why it was originally disabled (maybe the coloring issue with PowerShell?).
Fixes #2117